tools: add non-default OpenSSL versions to the test-shared workflow#62862
tools: add non-default OpenSSL versions to the test-shared workflow#62862panva wants to merge 10 commits intonodejs:mainfrom
Conversation
|
Review requested:
|
nodejs-github-bot
added
meta
panva
added
commit-queue-squash
|
Adding extra GHA workflows comes at the expense of spending more minutes to prepare security releases. We can skip the test-shared workflow on the private repo, but idk if it's worth it given that there are some path that we only run on that workflow. |
| # 3. Fetch OpenSSL release versions from endoflife.date, keep entries that | ||
| # are either not past EOL or still under extended support, then pick the | ||
| # first nix attr whose `.version` starts with the release version | ||
| # followed by `.` / letter / end-of-string (so "3.6" matches "3.6.1", | ||
| # "1.1.1" matches "1.1.1w", and "1.1" does NOT swallow "1.1.1"). | ||
| # Releases without a matching nix attr and the one covered by default in | ||
| # `build` are dropped. | ||
| curl -sf https://endoflife.date/api/openssl.json \ |
There was a problem hiding this comment.
Wouldn't it be better to have a manually curated list (by manually, I mean: the bot would open PRs to update the list every Sunday)? The logic is quite hard to grasp, and it's a lot of repetitive work that does not scale well if we wanted to add e.g. non-OpenSSL variants.
There was a problem hiding this comment.
uff, there's an idea, nixpgs actually has BoringSSL too. I'd do this in a follow-up.
There was a problem hiding this comment.
And it'd have to be the same job that does nixpkgs-unstable updates, else we'd run the risk of the two being open at the same time and not being in sync between their merges.
| --arg ccache "${NIX_SCCACHE:-null}" \ | ||
| --arg devTools '[]' \ | ||
| --arg benchmarkTools '[]' \ | ||
| ${{ endsWith(inputs.system, '-darwin') && '--arg withAmaro false --arg withLief false --arg withSQLite false --arg withFFI false --arg extraConfigFlags ''["--without-inspector" "--without-node-options"]'' \' || '\' }} |
There was a problem hiding this comment.
I think this would need to be moved to inputs.extra-nix-args
There was a problem hiding this comment.
I figured only extras (not shared between all uses of the composite action) would go through those inputs.
The OpenSSL versions are a big gap. Since we don't/can't keep up with the versions in CI. |
| build-openssl: | ||
| needs: | ||
| - build-tarball | ||
| - collect-openssl-versions |
There was a problem hiding this comment.
We might want to wait for the build job to finish, otherwise we'd have to run 5 times the same V8 version
There was a problem hiding this comment.
though it increases the total time waiting for the workflow since we even have to wait for the slow build ones. wdyt?
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
panva
force-pushed
the
additional-openssl-in-shared
branch
from
8bab015 to
1311e12
Compare
3 participants
Adds an additional OpenSSL shared-libraries matrix to
test-shared.ymlso PRs run against the supported OpenSSL releases we don't already build against by default. No more waiting for a full CI to find out a crypto/TLS change is broken on another version 🙏.build-opensslnow uses a committed matrix intest-shared.yml, regenerated by the existing weeklynixpkgs-unstableupdater so the pin bump and tested OpenSSL variants stay in sync in a single PR.buildjob already covers by default, while keeping ones that still have extended support such as1.1.1.SUPPORTED_OPENSSL_VERSIONremains a manually maintained value intest-shared.yml, and drives per-entrycontinue-on-errorin the autogenerated matrix so releases newer than what we explicitly support do not fail GHA.First commit for simple review, second is refactoring to a composite action, rest is fixes/applying feedback/minor refactors, and making the matrix be updated by the weekly job.